fix(driver-sql)!: findWithWindowFunctions presents its rows like every other read door (#16609) - #16716
Conversation
…er read door (#16609) The one record read door that returned `await builder` with no presentation: no `formatOutput` (every `find()`/`findOne()` row gets it) and no `presentReadValue` (`aggregate()`/`distinct()` got it under #3797/#3849). So a declared `Field.boolean` answered `1` where `find()` answered `true`, and a declared `Field.object` answered the stored JSON text where `find()` answered the parsed object. Each row now runs through the same `formatOutput` pass, minus the window function alias columns, which are computed values rather than declared fields. The collision case is ruled and pinned: an alias spelled the same as a declared field already won the key in SQL (`select *` plus `<window> as ok` keeps the last column), and its value now stays raw rather than being folded through the declared type's rule. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
📓 Docs Drift CheckThis PR changes 1 package(s): 2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 10 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin dce03cc6802761f1c99eeddb27ece57aab5e8442 && git checkout dce03cc6802761f1c99eeddb27ece57aab5e8442
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin b38821d1ce220527e2a7f34e254a96c48e2a9ba3 ea93dbea526f5a0084c742686dee5875951fe224 && git checkout -B drift-repro b38821d1ce220527e2a7f34e254a96c48e2a9ba3 && git merge --no-ff ea93dbea526f5a0084c742686dee5875951fe224
node scripts/docs-audit/affected-docs.mjs --json b38821d1ce220527e2a7f34e254a96c48e2a9ba3
|
Contract review (
|
| class | sqlite | postgres | mysql |
|---|---|---|---|
Field.boolean |
0/1 → false/true |
unchanged (native bool) | 0/1 → false/true (isSqlite || isMysql gate) |
Field.object / JSON |
JSON text → parsed object | unchanged (jsonb native) | unchanged (mysql2 parses JSON) |
| numeric fields | numeric string → number |
unchanged | unchanged |
Field.datetime + created_at/updated_at |
unchanged (already canonical text since #3912; naive-UTC repair applies) | after merge onto origin/main: Date → YYYY-MM-DDTHH:MM:SS.sssZ text (B1) |
same as postgres |
Field.date |
unchanged (toDateOnly on text is identity) |
unchanged (driver pins PG_OID_DATE parser to text, :5347) |
Date → YYYY-MM-DD text (toDateOnly, :16768) |
Field.time |
unchanged | canonical HH:MM:SS[.fff] via toTimeOnly |
same |
external.columnMap |
row key renames: remote column key → local field key (formatOutput :16717-16724, populated at :9605) |
same | same |
Alias columns: carved out, and the carve-out is sound — aliasIdentifierSql (:4826) wraps the alias through knex wrapIdentifier, so Postgres does not case-fold it and the row key equals String(wf.alias); the snapshot-and-restore at :9047-9060 therefore always finds the alias it protects.
3. Same presenter — confirmed, not a copy
sql-driver.ts:9058 calls this.formatOutput(object, row), the exact call findRows() makes at :5928; find()/findOne() apply nothing else per row. formatOutput / readPresentationKind / presentReadValue are called, never edited (hot-file fence held; git merge-tree origin/main refs/pull/16716/head is clean).
4. Governed paths
No. Three files: .changeset/window-functions-row-presentation.md, packages/drivers/driver-sql/src/sql-driver-window-function-output.test.ts, packages/drivers/driver-sql/src/sql-driver.ts. docs/adr/** untouched — so this is not the maintainer's merge on that ground. But see F4: the governed declaration goes stale the moment this merges.
5. Changeset — minor, and that is the repo-mandated level for a ! commit
.changeset/window-functions-row-presentation.md:2 grades @objectstack/driver-sql: minor with a **BREAKING** banner and an ADR-0087 not-required (no-migration-prescription) disposition. scripts/check-changeset-no-major.mjs:5-6, 41-45, 64-66 forbids major during the launch window and names the banner + disposition as the two carriers of breaking-ness. So ! + minor is compliant here, not a finding. FROM/TO coverage is incomplete — F2.
6. Tests / CI
- Pin that reddens on revert: yes —
presents a declared Field.boolean as a boolean, not 1/0and the JSON pin (sql-driver-window-function-output.test.ts:113-124), plus the three-way collision pin (:167-178); the PR's ablation shows 4 red. - CI on
8981967939: 46 check runs, allsuccessorskipped, none failed (Temporal Conformance, Test Core 6/6, Lint & Repo Gates, all four Type Check jobs, Governed Surface Queue Guard).mergeable_state: clean, draft.
Findings
F1 — wider than B1, with no ruling of its own on #16609. sql-driver.ts:9058 — routing through formatOutput moves the seven classes in §2, not the two B1 rules. That is what #16609 asks for and what triage adopted, so it is not blocking; but the maintainer signing Clause-②: yes should know the door moves booleans, JSON, numerics, Field.date, Field.time and columnMap keys, not only instants. Expectation: the PR body's "The fix" section lists all seven classes (it currently names boolean, JSON and the instants).
F2 — changeset FROM/TO incomplete. .changeset/window-functions-row-presentation.md:36-41 names boolean (1→true), object (JSON text → parsed) and the instants (TO given only by reference, "the same presented value find() gives"). Expectation: add FROM/TO lines for (a) external.columnMap — remote column key → local field key, every dialect; (b) SQLite numeric string → number; (c) MySQL Field.date Date → YYYY-MM-DD text; (d) Field.time → canonical HH:MM:SS[.fff]; and spell the instant TO as YYYY-MM-DDTHH:MM:SS.sssZ text on every dialect now that #16619 (45cfa1b88) is on main.
F3 — conformance cells on SQLite only. sql-driver-window-function-output.test.ts:52 — one describe, one in-memory SQLite driver; no DIALECT_CELLS / declareDialectCell arm although live-dialect-matrix.testkit.ts exists on the PR base and sql-driver-13973-canonical-iso-read-door.test.ts:118-124 shows the measure(cell) pattern. Consequence: the MySQL boolean half (the isSqlite || isMysql gate at :16751) and the PG/MySQL instant fold for this door are unmeasured — Temporal Conformance runs pnpm --filter @objectstack/driver-sql test with the live URLs, but this file has no live arm so it runs SQLite there too. The PR body's own admission stands: the five instant-agreement cases cannot fail on SQLite. Expectation: a measure(cell) over DIALECT_CELLS for this door asserting typeof row.ok === 'boolean' (MySQL cell) and expectCanonicalInstant for closed_at / created_at / updated_at (PG + MySQL cells).
F4 — the governed declaration contradicts the tree after merge. docs/adr/0053-date-and-datetime-semantics.md:1080-1081 ("findWithWindowFunctions is not one of these doors") and :1157-1159 ("Not covered: findWithWindowFunctions, which applies no read presentation of any kind today") on origin/main, plus sql-driver-13973-canonical-iso-read-door.test.ts:20-22, all become false when this merges — declared narrower than enforced, which is the inverse of the D-F addendum's stated purpose. Expectation: either fold a one-line D-F1 amendment into this PR (which makes it governed → the maintainer's merge), or file a docs-only governed card for the ADR line and link it in the PR body before merge; the test-header comment is non-governed and can be corrected here once the branch is on origin/main.
F5 — branch predates #16619. The head's last merge is of a0856e3bf9; origin/main has since taken 45cfa1b88 (#16619), whose formatOutput is the post-B1 presenter. merge-tree is clean, but every CI leg — including Temporal Conformance — measured this door through the pre-B1 formatOutput (:16745 at the head still gates the instants on isSqlite). Expectation: merge origin/main and re-run, so the "declared to CI" claim for PG/MySQL is measured against the presenter that will actually ship.
Not findings, recorded: no git stash, no governed edit, #13973 not folded in, #3797/#3849 not reopened; the collision ruling (alias wins the key, value stays raw) is pinned in code and test as triage demanded.
Generated by Claude Code
…FROM/TO table Contract-review patch round on PR #16716 (findings F2, F3, F5 and the non-governed half of F4). No production code changes. F5 — merged origin/main, so this branch now carries #16619: `formatOutput`'s instant gates are unconditional, which is the presenter this door actually ships through. Every CI leg on the previous head measured the pre-B1 presenter. F3 — `sql-driver-window-function-output.test.ts` gains a `measure(cell)` arm over `DIALECT_CELLS`, declared through `declareDialectCell` so an unprovisioned cell is a NAMED SKIP and never a silent pass. It asserts the two halves the SQLite-only arm cannot: `typeof row.ok === 'boolean'` (the MySQL half of the `isSqlite || isMysql` boolean gate) and the canonical `YYYY-MM-DDTHH:MM:SS.sssZ` text for `closed_at` / `created_at` / `updated_at` (the PG + MySQL instant fold). SS4 reads the same row back through raw knex to prove the fold is the driver's and not the client's. F2 — the changeset gains a per-class, per-dialect FROM/TO table covering all seven classes this door moves: adds `external.columnMap` (remote column key -> local field key, every dialect), the SQLite numeric-string -> `number` move, the MySQL `Field.date` `Date` -> `YYYY-MM-DD` move and `Field.time` -> canonical `HH:MM:SS[.fff]`, and spells the instant TO as the canonical text on every dialect. `minor`, the BREAKING banner and the ADR-0087 disposition are unchanged. F4 (non-governed half) — the header comment of `sql-driver-13973-canonical-iso-read-door.test.ts` said this door applies no read presentation. It routes through `formatOutput` since #16609, so the comment now says that and flags that ADR-0053 D-F1 still records it as not covered, with governed docs-only card #16782 carrying the amendment. `docs/adr/**` is untouched here. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TezFG8ZMrNH6n5VTNpPpdH
Patch round — F2, F3, F5 and the test-header half of F4 (director seat)New head: F5 — merged
|
| class | added FROM → TO |
|---|---|
external.columnMap |
the row KEY renames: remote column key → local field key, every dialect |
| numeric fields | numeric STRING off a legacy TEXT-affinity column → number (sqlite) |
Field.date |
Date → YYYY-MM-DD text (mysql) |
Field.time |
→ canonical HH:MM:SS[.fff]; PG's trimmed fraction re-padded ('09:30:00.5' → '09:30:00.500') |
| instants | TO spelled out as YYYY-MM-DDTHH:MM:SS.sssZ text on every dialect, never a Date |
unchanged rows are recorded rather than omitted — the same code path now runs for them, and silence there would read as "not considered". The two per-dialect claims that are not self-evident were re-verified in the merged tree rather than relayed: the PG date OID parser is pinned to text (sql-driver.ts:5418) and columnFieldByObject is populated at :9676 and consumed at :16849.
F4 — the non-governed half only
The header comment of sql-driver-13973-canonical-iso-read-door.test.ts said this door "applies no read presentation of any kind". Since #16609 it routes through formatOutput, so the comment now says that, points at the pin, and explicitly flags that ADR-0053 D-F1 still records the door as not covered with #16782 carrying the amendment — so a reader cannot mistake the stale ADR line for current behaviour. The governed ADR edit itself is not in this PR.
Test counts
pnpm --filter @objectstack/driver-sql exec vitest run src/sql-driver-window-function-output.test.ts
Test Files 1 passed (1)
Tests 17 passed | 2 skipped (19)
Was 12 passed / 0 skipped. The 2 skipped are the named live cells, verbatim from --reporter=verbose:
↓ sql-driver — window-function row presentation (#16609) matrix (live postgres)
> is provisioned — set OS_TEST_POSTGRES_URL to run this cell of the D-A3 driver axis
↓ sql-driver — window-function row presentation (#16609) matrix (live mysql)
> is provisioned — set OS_TEST_MYSQL_URL to run this cell of the D-A3 driver axis
Temporal Conformance (live PG + MySQL), which provisions both and runs pnpm --filter @objectstack/driver-sql test. The five SQLite cells of the new arm DID run and are green, so the arm is not vacuous even here.
Gates — exit codes verbatim
| command | exit |
|---|---|
pnpm --filter @objectstack/driver-sql exec vitest run src/sql-driver-window-function-output.test.ts |
0 |
pnpm --filter @objectstack/driver-sql typecheck |
0 (tsc --noEmit) |
node scripts/check-changeset-no-major.mjs --base origin/main |
0 |
node scripts/check-adr-0087-registration.mjs --base origin/main |
0 |
node scripts/check-empty-changeset.mjs --base origin/main |
0 |
pnpm check:nul-bytes |
0 |
pnpm check:test-source-alias |
0 |
pnpm check:cross-package-test-inputs |
0 |
Exit codes were captured by redirecting to a file before any pipe, never read through | tail. The three the review named print their own verdict lines:
✓ This diff introduces no `major` bump.
✓ check-adr-0087-registration: 1 declared-breaking changeset(s), each carrying an ADR-0087 disposition.
.changeset/window-functions-row-presentation.md [BREAKING+bang] not-required (no-migration-prescription)
✓ No empty-frontmatter changeset introduced by this diff (1 declaring changeset(s) added).
The last two are the ones that could have reddened on an F2 edit — the banner and the disposition survived the rewrite, which is what they assert.
check-changeset-no-major additionally reports LEVEL AXIS: NOT MEASURED locally (no pull_request payload was available to read a declaration from). That is the same local/--event split the original body already recorded as noted, not filed; it is neither a pass nor a failure here, and CI's --event run is what judges the LEVEL axis.
dispatch-gates --commands derives 57 families for this change set (unchanged from the reviewed head — the same four paths, no new file kind). The eight above are the ones this patch round could move; the remaining families and the repo-wide pnpm lint are CI's run, not this seat's.
Not done, and why
- F1's code half — nothing to do: F1 is an expectation on the PR body, and the body's "The fix" section now enumerates all seven classes with the ADR-0053 D-F1 / docs(adr-0053): D-F1 says
findWithWindowFunctionsapplies no read presentation — false once #16716 merges (governed, docs-only) #16782 note. The body edit landed; GitHub appended its bare-form footer beside the original session-URL one, which is the documented behaviour of aPATCHand not a mutation. - F4's governed half —
docs/adr/0053-date-and-datetime-semantics.md:1080-1081, :1157-1159deliberately untouched. Governed, docs-only, maintainer merge: docs(adr-0053): D-F1 saysfindWithWindowFunctionsapplies no read presentation — false once #16716 merges (governed, docs-only) #16782. - One cosmetic typo in the commit body (
SS4where§L4was meant). Left as-is: amending was excluded by this round's dispatch, the branch squashes on merge, and the section names are correct in the file itself and above.
Generated by Claude Code
Contract review (
|
| # | prior expectation | status | evidence |
|---|---|---|---|
| F1 | PR body "The fix" lists all seven classes | discharged | body §"The seven column classes this door now moves" — 7-row table (boolean, object/JSON, numeric, instants incl. audit stamps, Field.date, Field.time, external.columnMap); names #16782 |
| F2 | changeset FROM/TO covers columnMap, SQLite numeric, MySQL Field.date, Field.time; instant TO spelled |
discharged in content — but the discharge reddens a required gate (R1) | .changeset/window-functions-row-presentation.md:35-53 — 7-row × 3-dialect table; :50 "YYYY-MM-DDTHH:MM:SS.sssZ TEXT on every dialect, never a JS Date"; minor, **BREAKING**, adr-0087: not-required (no-migration-prescription) all intact |
| F3 | measure(cell) over DIALECT_CELLS, typeof row.ok === 'boolean', canonical instants for closed_at/created_at/updated_at, named skips |
discharged | verification 3 |
| F4 (test-header half) | sql-driver-13973-canonical-iso-read-door.test.ts header corrected, ADR line flagged as stale, #16782 named |
discharged | verification 4 |
| F4 (governed half) | ADR-0053 D-F1 amendment | not in this PR, by design — card #16782 | docs/adr/** absent from the diff (verification 1) |
| F5 | merge origin/main post-#16619 and re-measure |
discharged | merge commit 40b7cd4ff of ed7243d52; head's formatOutput (sql-driver.ts:16843) is the post-B1 presenter (presentAuditTimestampOutput at :16943, the instant fold outside any isSqlite gate at :16926-16952); Temporal Conformance re-ran on this head — verification 7 |
Verification
-
Files.
git diff ed7243d52..d257234a7 --name-status:A .changeset/window-functions-row-presentation.md,M packages/drivers/driver-sql/src/sql-driver-13973-canonical-iso-read-door.test.ts,A packages/drivers/driver-sql/src/sql-driver-window-function-output.test.ts,M packages/drivers/driver-sql/src/sql-driver.ts— four files.docs/adr/**,content/docs/releases/**,packages/spec/**: absent. The 156-file8981967..d257234range isorigin/main's own motion arriving through the merge, not this PR's. Two commits on the branch since the prior head: the merge andd257234a7. -
Changeset. Frontmatter
"@objectstack/driver-sql": minor; the**BREAKING**banner and the single<!-- adr-0087: not-required (no-migration-prescription) … -->marker are byte-unchanged from8981967(the diff between the two heads touches only the body: the new §"What moves" heading + table + two paragraphs, one addedNumber(row.amount)bullet, one bullet reworded, one columnMap bullet). Per-dialect claims spot-checked against the head'ssql-driver.ts: boolean gateisSqlite || isMysql(:16861arm + MySQL arm), JSON parse underisSqlite(:16897), numeric-string fold (:16916), PGdateOID parser pinned to text (:5418),columnFieldByObjectpopulated:9676/ consumed:16849-16851. Content correct. Gate outcome: R1. -
Live-dialect arm (
sql-driver-window-function-output.test.ts:227-380). ImportsDIALECT_CELLS,declareDialectCell,assertThreeWayZoneSkew,readServerZone,type DialectCellfrom./live-dialect-matrix.testkit.js(:50-56);for (const cell of DIALECT_CELLS) declareDialectCell(cell, 'window-function row presentation (#16609)', measure)(:378-380).declareDialectCell(testkit:506-540) routes an unprovisioned cell todeclareUnprovisionedCell, whoseit.skipIf(!EXPECT_LIVE_DIALECTS)is a named skip locally and anexpect.failRED underOS_EXPECT_LIVE_DIALECT_MATRIX=1— no silent path. §L1expect(typeof row.ok).toBe('boolean')+[true, false](:317-325); §L2expectCanonicalInstantoverclosed_at/created_at/updated_at— notDate,typeof 'string',/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/— andclosed_atequal to the two seeded ISO literals (:327-335); §L4 reads the raw row through(driver as any).knex(LIVE_TABLE)…first()and asserts the instants areDateand MySQLokis anumberoff the client (:349-371), which is what makes §L1/§L2 a measurement offormatOutputrather than of the client. Live cells runassertThreeWayZoneSkew(:269). §L0 non-vacuity (:305-315), §L3 door agreement (:337-347). -
13973 header.
sql-driver-13973-canonical-iso-read-door.test.ts:20-28now: "Since driver-sql:findWithWindowFunctionsreturns storage forms — a declared boolean answers1and an object field answers JSON text wherefind()answerstrueand the parsed object #16609 it routes each row through the SAMEformatOutputpassfind()runs (minus the window-alias columns) … pinned bysql-driver-window-function-output.test.ts…⚠️ ADR-0053 D-F1 still RECORDS that door as not covered … docs-only governed card docs(adr-0053): D-F1 saysfindWithWindowFunctionsapplies no read presentation — false once #16716 merges (governed, docs-only) #16782 carries the amendment." Nothing else in the file moved. -
sql-driver.tshunk unchanged.git diff a0856e3bf..8981967vsgit diff ed7243d52..d257234a7on the file,@@/indexlines stripped: byte-identical — one hunk,return await builder;→ rows + snapshot /this.formatOutput(object, row)/ restore. It now sits at:9074-9133(call at:9129, was:9058) purely becauseorigin/maingrew above it. Blob differs from8981967only by the merged main content.formatOutput/readPresentationKind/presentReadValuestill called, never edited. -
PR body. "The fix" carries the seven-class table with per-dialect "where the row actually changes"; the ADR-0053 D-F1 / docs(adr-0053): D-F1 says
findWithWindowFunctionsapplies no read presentation — false once #16716 merges (governed, docs-only) #16782 note; the "Superseded by the patch round" note on the conformance counts (17 passed | 2 skipped). -
CI on
d257234a7(33 check runs, read 05:38Z):Check Changeset— failure (R1).Temporal Conformance (live PG + MySQL)— success (05:31–05:37Z). Success: Build Core, Test Core 1/6, Dogfood Verify CLI, Type Check · source gates / debt ledger / consumer gates, Governed Surface Queue Guard, Check Documentation Links, Flag docs, Check PR Size, Auto Label, the three claim guards ×2. Skipped: Build Docs, Console Pin Gate, Packed-tarball smoke. Still in progress: Test Core 2–6/6, Dogfood Regression Gate 1–3/3, Lint & Repo Gates, Type Check · workspace.
On the live cells having actually run: the job'sdriver-sqlstep (ci.ymlat the head, jobTemporal Conformance (live PG + MySQL), step "Run driver-sql suite against both live servers") setsOS_TEST_POSTGRES_URL,OS_TEST_MYSQL_URLandOS_EXPECT_LIVE_DIALECT_MATRIX: '1', runspnpm --filter @objectstack/driver-sql test(= barevitest run, no include/exclude), so an unprovisioned#16609cell would have been a RED, not a skip; success therefore means §L0–§L4 passed on live PG and live MySQL. Stated as an inference from job config + conclusion: the log API returns only the last 5000 of 11027 lines and thedriver-sqlblock is in the first half I could not retrieve.
Residual findings
R1 — Check Changeset red on the head; the F2 discharge is the cause. Blocking. check-adr-0087-registration --base ed7243d52 (job 101948563951, 05:31:25Z): "not-required (no-migration-prescription) contradicts the changeset's own body, which carries a migration prescription. Evidence (from-to-label): **What moves, FROM → TO, per column class and per dialect.** Routing this door". Reproduced with the gate's own export: scripts/check-adr-0087-registration.mjs is the same blob at the head and at the merge base (ebc55c70cc), and findMigrationPrescription() from that file returns null for the 8981967 body, {branch:'from-to-label', line:'**What moves, FROM → TO, …'} for the d257234 body, and null again for the d257234 body with only that heading reworded (**What moves, per column class and per dialect (storage form → presented form).**). The table rows (1 / 0 → true / false, Date → '2026-01-10T…'), the → rewrite bullets and the word "renames" in the columnMap row do not trip any branch — the prior head already carried the arrow bullets and was green. Only the literal FROM … → … TO used as a label does (FROM_TO_LABEL_RE :1132; labelPositioned :1308 — a comma before FROM is not a governing word). So the patch-round line "check-adr-0087-registration.mjs --base origin/main → exit 0" does not reproduce against this head; CI is authoritative. Expectation: reword that one heading so the placeholder token is not used as a label (the reworded form above verifies null; do not drop the **BREAKING** token or switch category — the gate's own text forbids both routes here). No other file moves.
R2 — nits, non-blocking. (a) sql-driver-window-function-output.test.ts:49 imports '../src/index.js' from inside src/ — resolves to the same file, but the sibling convention is './index.js' (sql-driver-13973-…:71). (b) The same file's header :14-21 and :168-171 still describe the instant presentation as "under change (ADR-0053 D-F1)"; #16619 has landed and is merged here, so the SQLite arm's agreement-not-literal rationale is now historical — the ## Two arms section (:26-45) states the current position correctly, so this is wording, not substance.
Not findings, recorded: no governed path in the diff; no git stash (stash list empty); #13973 not folded in; #3797 / #3849 not reopened; the alias-collision ruling remains pinned in code (sql-driver.ts:9094-9115) and test (:208-217).
Blocks landing besides #16782: R1 only — a one-line changeset heading edit, then Check Changeset must be green on the new head (and the still-running jobs must finish green).
Generated by Claude Code
…s no prescription (#16609) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TezFG8ZMrNH6n5VTNpPpdH
Patch round 2 — R1 (director seat)R1 is fixed on a new head; nothing else moved. One file, one line. The reword (
The Detector reading, both legs.
The control leg is why the Gates, exit codes captured before any pipe, run in the worktree at the new head with
The first one was 1 on New head: R2 left alone, deliberately. (a) CI on the new head is not waited on here. Generated by Claude Code |
Contract review — R1 discharged at
|
…d door PR #16716 (card #16609) routes findWithWindowFunctions rows through the same `formatOutput` pass `find()` runs, so ADR-0053's three statements that the door applies no read presentation are false on the tree as of that merge. Three carriers amended, all prose, all re-derived by symbol: - `:3` the Status line — the "but `findWithWindowFunctions`" exception, which is the ADR's summary for a reader who never opens the addendum - `:1081` D-F1's body — "is not one of these doors" - `:1157` the Consequences bullet — "Not covered: ... applies no read presentation of any kind today" Each amendment states separately what the door moves (the columnMap row-KEY rename, JSON, numeric strings, the two instant classes, boolean, date, time) and which of it D-F1 governs (the two instant classes only), so the correction does not replace one overstatement with another. The window ALIAS carve-out is recorded at each site: a computed alias wins the key and its value stays raw. D-F3's Invalid `Date` carve-out is untouched, verbatim, at both sites that carry it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
Fixes #16609
Clause-②: yes— this changes the shape of the payload a published driver door returns.The defect
SqlDriver#findWithWindowFunctionswas the one record read door that returnedawait builderwith no presentation at all: noformatOutput(which everyfind()/findOne()row gets) and nopresentReadValue(whichaggregate()/distinct()got under #3797 / #3849). So it handed back storage forms where every other door hands back the declared type's presentation.Reproduced first, on this branch's own build
The card's probe, run verbatim against
packages/drivers/driver-sql/dist/index.jsbuilt fromorigin/main2e6a2ea4c9— before any code change. Not relayed from the card's transcript:After the fix, the same probe on the same rebuilt
dist:Symbols, re-derived (every anchor on the card had drifted)
Read authoritatively with
git show origin/main:packages/drivers/driver-sql/src/sql-driver.ts(17582 lines):origin/main2e6a2ea4c9findWithWindowFunctions:8961, terminal statementreturn await builder;at:9006distinct's presentation (the firing control):8944–:8946readPresentationKind/presentReadValue/formatOutput:12874/:12901/:16655— called, never editedThe fix
Each row runs through the same
formatOutputpassfind()runs, minus the window-function alias columns.The seven column classes this door now moves
formatOutputis one pass over seven rules, so routing through it moves all seven — not only the boolean and JSON classes the defect was reported as. Enumerated so the reviewer signingClause-②: yessees the whole payload change, not a subset (contract review F1):Field.boolean1/0→true/falseisSqlite || isMysqlgate; PG stores a nativeboolean)Field.object(JSON)jsonband mysql2 already parse)numbercreated_at/updated_atand every declaredField.datetimeDate→ canonicalYYYY-MM-DDTHH:MM:SS.sssZtextField.dateDate→YYYY-MM-DDtextdateOID parser to text)Field.timeHH:MM:SS[.fff], re-padding the fraction PG trimsexternal.columnMapSix of the seven are what #16609 asks for in so many words ("the same presentation the other doors apply"). The seventh —
external.columnMap— nobody named on the card; it is a KEY move rather than a value move, and it is listed here rather than discovered at merge. All seven now carry FROM/TO lines in the changeset.docs/adr/0053-date-and-datetime-semantics.md:1080-1081,:1157-1159) still says this door is not covered; docs-only governed card #16782 carries the amendment. The tree is ahead of the declaration on that line until #16782 lands — declared narrower than enforced. The non-governed half of the same staleness (the header comment ofsql-driver-13973-canonical-iso-read-door.test.ts) is corrected in this PR.formatOutputrather thanpresentReadValue, and that choice is load-bearing:ReadPresentationKindis'datetime' | 'date' | 'time' | 'boolean' | 'number'— it has nojsonmember, so the per-value helper the other two doors use cannot present a declaredField.objectat all. These are rows, which is exactly whatformatOutputtakes.The carve-out is snapshot-and-restore rather than a "which keys would
formatOutputtouch?" pre-computation, because that question can only be answered by re-reading the declared-field registriesformatOutputreads — a second, worse copy that would go stale the next timeformatOutputlearns a rule.The alias / declared-field collision — ruled and pinned
An alias wins the key, and its value stays raw.
SQL had already decided the first half before the driver sees it:
select *plus a window function aliasedas okprojects two columns namedok, and the row object keeps the last — so the computed value wins and the declared column's value is not in the row at all. Measured onorigin/main2e6a2ea4c9withalias: 'ok'over a declaredField.boolean okseededtrue/false: rows came backok: 1andok: 2— the ROW_NUMBERs. That is unchanged by this PR.What this PR rules is the second half: the winning value is not presented as the declared type. Folding ROW_NUMBER
1and2through the boolean rule would yieldtrueandtrueand destroy the value the caller asked for.This matches the ruling
aggregate()already makes for a date-bucketed column aliased as its own field name ("leaves a date-BUCKETED column as its label, not an instant").⭐ Pinned as a test, not prose —
sql-driver-window-function-output.test.ts, the case "a colliding alias wins the key AND keeps its raw computed value", whose assertion can fail three distinguishable ways — plus the in-code comment block at the fix site.Conformance case
New
packages/drivers/driver-sql/src/sql-driver-window-function-output.test.ts(12 cases) asserts door-to-door agreement —findWithWindowFunctions()versusfind()on the same row — across boolean, json, date, time, datetime and the audit stamps.formatOutputproduces for the instant classes (ADR-0053 D-F1). Both doors run the same pass, so they move together and this stays true whichever way that lands. Booleans and JSON are additionally pinned absolutely — they are wrong on SQLite today and #16619 does not touch them.📌 Superseded by the patch round (contract review F3 / F5). #16619 has since landed on
mainand is merged in here, soformatOutputis now the post-B1 presenter; the file gained ameasure(cell)arm overDIALECT_CELLSthat asserts the instant shape absolutely on the live cells, where the fold is real work. The counts below are the pre-patch-round ones — the current file is 17 passed | 2 skipped (19), the 2 being the named live-PG / live-MySQL skips. See the patch-round comment on this PR.Two-leg ablation
formatOutputcall ablated at the window-door site only (thefindRowssite at:5928left intact, verified by anchor count 2 → 1): 4 failed | 8 passed, e.g.AssertionError: typeof ok on row 0: expected 'number' to be 'boolean'.git hash-objectback to057310aead2557a750e4884f3a3434c55b7c21f1, equal to the HEAD blob, withgit diff HEADempty. The mutation was proven to land on disk first (hash changed, injected marker present) so the leg could not be a silent no-op.8 cases stayed green in leg 2, and that is reported rather than glossed. The layer holding them is canonical-on-write storage, measured directly on the raw SQLite table:
Since #3912 the instant classes are stored canonically on SQLite, so removing the read presentation moves nothing for them — those assertions cannot fail on this dialect today. They still earn their place: they hold the door-to-door invariant on live Postgres and MySQL, where the client library returns
Dateobjects, and those arms are skipped in this container (see below).Verification
pnpm --filter @objectstack/driver-sql test158 passed | 10 skipped (168)files,2414 passed | 141 skipped (2555)testspnpm --filter @objectstack/driver-sql typechecktsc --noEmit, read out ofpackage.jsonOS_TEST_POSTGRES_URL/OS_TEST_MYSQL_URL, absent in this container. That is load-bearing here: the boolean rule fires on SQLite and MySQL (formatOutputgatesisSqlite || isMysql), so the MySQL half of this fix is not measured locally and is declared to CI.The
typecheckleg genuinely covers the new test:tsc --noEmit --listFilesnamessql-driver-window-function-output.test.ts(count 1, not 0), and the package'stsconfig.jsonhasinclude: ["src/**/*"]with no test exclusion.Gates
Derived with
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack— 57 families, all run, reconciled at the final head:55 exited 0. Two exited 3 —
PREREQUISITE NOT MET, which is not a pass and not a failure:check:dual-build-cjs-loads— "this gate reads built output, and some package has no dist/ … 79 packages … ⛔ This is NOT a pass: nothing was measured."check:type-check-debt— same class.Both need a whole-repo
pnpm build, which is CI's run rather than this seat's. Recorded as NOT MEASURED, declared to CI.Repo-wide
pnpm lintis likewise CI's run, not owed here.Changeset grade — derived from the repo, and where it is genuinely ambiguous
Graded
minoron@objectstack/driver-sql, with a**BREAKING**banner and an ADR-0087 disposition.check-adr-0087-registrationaccepts it:Precedent found, reading the published CHANGELOG rather than a remembered table — both entries are in
packages/drivers/driver-sql/CHANGELOG.md, both in 17.0.0:### Patch Changes### Minor Changes**BREAKING**banner and no ADR-0087 marker in its 88-line entryThat zero-hit has a firing positive control: the same file carries
BREAKING9 times andadr-008717 times elsewhere, so markers demonstrably survive into CHANGELOGs and the absence is real. #3849 declared breaking through the third spelling instead — the conventional-commit!in its summary.WHICH LEVELruling (pr-automation.yml:667-682, maintainer, 2026-09-04, decision batch #35 on #15294) states "The 64 historicalpatchprecedents are pre-rule and nothing is retro-fixed."Under the current rule the case is genuinely ambiguous, and this is flagged for the reviewer rather than smoothed over:
index, a new accepted key or value" → at leastminor. This PR adds none of those.fix(that changes no public surface stayspatch". This PR does change a public surface (the returned payload's value types) without widening it — so it sits cleanly in neither bucket.patchchangeset.minorwas chosen because gradingpatchwhile declaringClause-②: yesis exactly the "self-contradiction inside one PR" the LEVEL axis names, and because #3849 — the same defect class on the same door family — choseminor. No gate forbidsminor;patchwould be forbidden if the axis could see this package. ⇒ If the reviewer reads the surface as unchanged, lowering topatchis a one-line edit.packages/*/src/**pattern matches one segment, sopackages/drivers/driver-sql/src/**never matches;packagesTouchedreturns{ packages: [], unreadable: [] }for this diff. Verified with a control: grading this changesetpatchunder aClause-②: yespayload stayed green. Filed separately as #16713 (51 of 74 workspace packages affected) — a distinct axis from the sibling #16692, which covers non-srcroots at depth 1.Scope
Hot-file fence held. This PR touches
findWithWindowFunctionsand its body only;formatOutput/readPresentationKind/presentReadValueare called, never edited, andgit diff origin/mainconfirmssql-driver.tsdid not move for #15546's or #16570's regions. #13973 is not folded in and #3797 / #3849 are not reopened.验收备注
.claude/scripts/dispatch-gates.mjs不存在;实际路径是scripts/pm/dispatch-gates.mjs,已按 agent 文件为准使用并回报 PM。noted, not filed:check-adr-0087-registration的--base origin/main读数与--event读数在本地互不相干,两者都绿,但只有后者能判 LEVEL 轴 —— 这只是观察,不构成缺陷。🤖 Generated with Claude Code
https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
Generated by Claude Code
Generated by Claude Code